Skip to content

RHINENG-25944: add search to the tags list endpoint#2204

Open
dominikvagner wants to merge 1 commit into
RedHatInsights:masterfrom
dominikvagner:tags-search
Open

RHINENG-25944: add search to the tags list endpoint#2204
dominikvagner wants to merge 1 commit into
RedHatInsights:masterfrom
dominikvagner:tags-search

Conversation

@dominikvagner
Copy link
Copy Markdown
Contributor

@dominikvagner dominikvagner commented May 21, 2026

Summary

Allow /tags to filter returned tags by matching the search term against tag namespace, key, and value.
With this the /tags endpoint could be used in the frontend for tag filtering.

Summary by Sourcery

Add search support to the system tags listing endpoint and document the new query parameter.

New Features:

  • Allow filtering system tags by a search term matching tag namespace, key, or value via the /tags endpoint.

Documentation:

  • Document the search query parameter for the system tags list endpoint in the API documentation.

Tests:

  • Add tests covering successful tag search results and empty results when no tags match the search term.

Allow `/tags` to filter returned tags by matching the search term
against tag namespace, key, and value.
With this the `/tags` endpoint could be used in the frontend for tag
filtering.
@dominikvagner dominikvagner requested a review from a team as a code owner May 21, 2026 15:37
@dominikvagner dominikvagner changed the title RHINENG-25944: add search for the tags list endpoint RHINENG-25944: add search to the tags list endpoint May 21, 2026
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 21, 2026

Reviewer's Guide

Adds search support to the /tags listing endpoint by wiring search fields into the generic list options, documenting the new query parameter, and covering it with tests for matching and non-matching queries.

File-Level Changes

Change Details Files
Enable full-text-style search on the /tags endpoint across namespace, key, and value fields via existing list options.
  • Extend SystemTagsOpts with SearchFields pointing to namespace, key, and value JSON properties in the underlying SQL query.
  • Rely on the existing generic listing/search infrastructure to filter tags returned by SystemTagListHandler based on the search query parameter.
manager/controllers/systemtags.go
Document and validate behavior of the new search capability for the system tags listing endpoint.
  • Add unit tests that assert correct filtering, counts, and metadata (TotalItems and Search) when searching for an existing tag key.
  • Add a unit test verifying that unknown search terms return an empty data set while still echoing the search term in response metadata.
  • Update the OpenAPI v3 documentation for the /tags endpoint to describe the new search query parameter and align formatting of existing query param docs.
manager/controllers/systemtags_test.go
docs/v3/openapi.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="manager/controllers/systemtags.go" line_range="52-55" />
<code_context>
 	},
 	StableSort:  "tag",
 	DefaultSort: "tag",
+	SearchFields: []string{
+		"sq.tag->>'namespace'",
+		"sq.tag->>'key'",
+		"sq.tag->>'value'",
+	},
 }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid hard-coding the SQL alias in `SearchFields` to reduce coupling and future breakage risk

These `SearchFields` are tightly coupled to the `sq` alias (for example, `"sq.tag->>'namespace'"`). If the table/CTE alias changes, search will silently break. Either make these expressions independent of the outer alias (if supported), or centralize them in a shared constant/helper used by both the query builder and `SearchFields` so alias updates are done in one place.

Suggested implementation:

```golang
	SearchFields: []string{
		"tag->>'namespace'",
		"tag->>'key'",
		"tag->>'value'",
	},

```

This change assumes that the query builder / underlying SQL does not require an explicit table/CTE alias prefix for these expressions. If the current implementation *does* require the alias (e.g. because multiple tables expose a `tag` column), you should instead:
1. Introduce a shared constant or helper (e.g. `const systemTagAlias = "sq"` or a function that formats search fields for a given alias) in this file or a shared package.
2. Use that constant/helper both where the alias is defined in the SQL/CTE and here in `SearchFields`, so alias changes are centralized.
</issue_to_address>

### Comment 2
<location path="manager/controllers/systemtags_test.go" line_range="81-90" />
<code_context>
 	assert.Equal(t, 400, w.Code)
 }
+
+func TestSystemTagsListSearch(t *testing.T) {
+	core.SetupTest(t)
+
+	w := CreateRequestRouterWithAccount("GET", "/", "", "?search=k3", nil, "", SystemTagListHandler, 1)
+
+	var output SystemTagsResponse
+	CheckResponse(t, w, http.StatusOK, &output)
+
+	assert.Equal(t, 2, len(output.Data))
+	assert.Equal(t, 2, output.Meta.TotalItems)
+	assert.Equal(t, "k3", output.Meta.Search)
+
+	assert.Equal(t, 1, output.Data[0].Count)
+	assert.Equal(t, "k3", output.Data[0].Tag.Key)
+	assert.Equal(t, "ns1", output.Data[0].Tag.Namespace)
+	assert.Equal(t, "val3", output.Data[0].Tag.Value)
+
+	assert.Equal(t, 3, output.Data[1].Count)
+	assert.Equal(t, "k3", output.Data[1].Tag.Key)
+	assert.Equal(t, "ns1", output.Data[1].Tag.Namespace)
+	assert.Equal(t, "val4", output.Data[1].Tag.Value)
+}
+
</code_context>
<issue_to_address>
**suggestion (testing):** Avoid relying on result ordering and tighten assertions to better express expectations

This test is tightly coupled to the current result order and exact per-index counts/values, so it may become brittle if query or default sorting changes while behavior remains correct. Either (a) assert that both expected tag combinations are present regardless of order (e.g., by sorting or mapping the response), or (b) treat sort order as part of the contract and assert on the sort field explicitly. Also consider asserting `output.Meta.TotalItems` relative to `len(output.Data)` to ensure pagination metadata matches the returned items.

Suggested implementation:

```golang
import (
	"fmt"
	"net/http"
	"testing"

	"app/base/core"

	"github.com/stretchr/testify/assert"
)

```

```golang
func TestSystemTagsListSearch(t *testing.T) {
	core.SetupTest(t)

	w := CreateRequestRouterWithAccount("GET", "/", "", "?search=k3", nil, "", SystemTagListHandler, 1)

	var output SystemTagsResponse
	CheckResponse(t, w, http.StatusOK, &output)

	// Expected tag combinations and counts, independent of result order.
	expected := map[string]int{
		"ns1:k3:val3": 1,
		"ns1:k3:val4": 3,
	}

	assert.Equal(t, len(expected), len(output.Data))
	assert.Equal(t, len(expected), output.Meta.TotalItems)
	assert.Equal(t, "k3", output.Meta.Search)

	seen := make(map[string]bool)
	for _, item := range output.Data {
		key := fmt.Sprintf("%s:%s:%s", item.Tag.Namespace, item.Tag.Key, item.Tag.Value)

		expectedCount, ok := expected[key]
		assert.True(t, ok, "unexpected tag combination in response: %s", key)
		assert.Equal(t, expectedCount, item.Count, "unexpected count for tag %s", key)

		seen[key] = true
	}

	// Ensure all expected tag combinations were returned.
	for key := range expected {
		assert.True(t, seen[key], "expected tag combination not found in response: %s", key)
	}
}

```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +52 to +55
SearchFields: []string{
"sq.tag->>'namespace'",
"sq.tag->>'key'",
"sq.tag->>'value'",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Avoid hard-coding the SQL alias in SearchFields to reduce coupling and future breakage risk

These SearchFields are tightly coupled to the sq alias (for example, "sq.tag->>'namespace'"). If the table/CTE alias changes, search will silently break. Either make these expressions independent of the outer alias (if supported), or centralize them in a shared constant/helper used by both the query builder and SearchFields so alias updates are done in one place.

Suggested implementation:

	SearchFields: []string{
		"tag->>'namespace'",
		"tag->>'key'",
		"tag->>'value'",
	},

This change assumes that the query builder / underlying SQL does not require an explicit table/CTE alias prefix for these expressions. If the current implementation does require the alias (e.g. because multiple tables expose a tag column), you should instead:

  1. Introduce a shared constant or helper (e.g. const systemTagAlias = "sq" or a function that formats search fields for a given alias) in this file or a shared package.
  2. Use that constant/helper both where the alias is defined in the SQL/CTE and here in SearchFields, so alias changes are centralized.

Comment on lines +81 to +90
func TestSystemTagsListSearch(t *testing.T) {
core.SetupTest(t)

w := CreateRequestRouterWithAccount("GET", "/", "", "?search=k3", nil, "", SystemTagListHandler, 1)

var output SystemTagsResponse
CheckResponse(t, w, http.StatusOK, &output)

assert.Equal(t, 2, len(output.Data))
assert.Equal(t, 2, output.Meta.TotalItems)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Avoid relying on result ordering and tighten assertions to better express expectations

This test is tightly coupled to the current result order and exact per-index counts/values, so it may become brittle if query or default sorting changes while behavior remains correct. Either (a) assert that both expected tag combinations are present regardless of order (e.g., by sorting or mapping the response), or (b) treat sort order as part of the contract and assert on the sort field explicitly. Also consider asserting output.Meta.TotalItems relative to len(output.Data) to ensure pagination metadata matches the returned items.

Suggested implementation:

import (
	"fmt"
	"net/http"
	"testing"

	"app/base/core"

	"github.com/stretchr/testify/assert"
)
func TestSystemTagsListSearch(t *testing.T) {
	core.SetupTest(t)

	w := CreateRequestRouterWithAccount("GET", "/", "", "?search=k3", nil, "", SystemTagListHandler, 1)

	var output SystemTagsResponse
	CheckResponse(t, w, http.StatusOK, &output)

	// Expected tag combinations and counts, independent of result order.
	expected := map[string]int{
		"ns1:k3:val3": 1,
		"ns1:k3:val4": 3,
	}

	assert.Equal(t, len(expected), len(output.Data))
	assert.Equal(t, len(expected), output.Meta.TotalItems)
	assert.Equal(t, "k3", output.Meta.Search)

	seen := make(map[string]bool)
	for _, item := range output.Data {
		key := fmt.Sprintf("%s:%s:%s", item.Tag.Namespace, item.Tag.Key, item.Tag.Value)

		expectedCount, ok := expected[key]
		assert.True(t, ok, "unexpected tag combination in response: %s", key)
		assert.Equal(t, expectedCount, item.Count, "unexpected count for tag %s", key)

		seen[key] = true
	}

	// Ensure all expected tag combinations were returned.
	for key := range expected {
		assert.True(t, seen[key], "expected tag combination not found in response: %s", key)
	}
}

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.23%. Comparing base (c733d57) to head (fdae18e).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2204   +/-   ##
=======================================
  Coverage   59.23%   59.23%           
=======================================
  Files         137      137           
  Lines        8795     8795           
=======================================
  Hits         5210     5210           
  Misses       3037     3037           
  Partials      548      548           
Flag Coverage Δ
unittests 59.23% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rverdile rverdile self-assigned this May 21, 2026
Comment on lines +52 to +55
SearchFields: []string{
"sq.tag->>'namespace'",
"sq.tag->>'key'",
"sq.tag->>'value'",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question for my understanding, how did you determine these 3 fields should be searched?

Copy link
Copy Markdown
Contributor

@rverdile rverdile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe @MichaelMraka or @TenSt can double check but this lgtm! Working locally for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants